Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[MUST BE CAREFULLY REVIEWED] saltcloud / lxc improvments #10869

Merged
merged 18 commits into from Mar 6, 2014
Merged

[MUST BE CAREFULLY REVIEWED] saltcloud / lxc improvments #10869

merged 18 commits into from Mar 6, 2014

Conversation

kiorky
Copy link
Contributor

@kiorky kiorky commented Feb 28, 2014

If you want to begin review, feel free but it is not finished.

Idea is to complete lxc modules , fix some salt cloud code (multiprocessing) and add a saltcloud lxc provider

@kiorky
Copy link
Contributor Author

kiorky commented Feb 28, 2014

/cc @thatch45 @regilero

@ghost
Copy link

ghost commented Feb 28, 2014

Test FAILed.
Refer to this link for build results: http://jenkins.saltstack.com/job/salt-pr-build/1915/

@ghost
Copy link

ghost commented Feb 28, 2014

Test FAILed.
Refer to this link for build results: http://jenkins.saltstack.com/job/salt-pr-build/1916/

@ghost
Copy link

ghost commented Mar 1, 2014

Test FAILed.
Refer to this link for build results: http://jenkins.saltstack.com/job/salt-pr-build/1923/

@techhat
Copy link
Contributor

techhat commented Mar 1, 2014

@s0undt3ch, would you please take a close look at this?

@s0undt3ch
Copy link
Member

@techhat I sure will, though, once I have the necessary time available... @kiorky don't despair!

@mgwilliams
Copy link
Contributor

Note that lxc userspace tools v1.0 was released last week. It is a complete overhaul of the tools, and I think packages are going to be aggressively pushed out for current OS releases. While it is mostly backwards compatible, it did break the salt lxc module. I mention it here, as some of the updates for 1.0 (particularly the lxc.info function) would not be incompatible with this pull request. I should have my updates pushed out by end of day on Monday.

@kiorky
Copy link
Contributor Author

kiorky commented Mar 1, 2014

what im doing is 1.0 only anyway. :)

@kiorky
Copy link
Contributor Author

kiorky commented Mar 1, 2014

And, im thinking im now at 75%/80% done.

@kiorky
Copy link
Contributor Author

kiorky commented Mar 1, 2014

(i think i wont have time to add much tests for the moment)

@kiorky
Copy link
Contributor Author

kiorky commented Mar 1, 2014

rebased my current working copy, there must be debug and pdb in there ^^

@kiorky
Copy link
Contributor Author

kiorky commented Mar 1, 2014

Strangely github doesnt show commits in the order of my git log does

@ghost
Copy link

ghost commented Mar 1, 2014

Test FAILed.
Refer to this link for build results: http://jenkins.saltstack.com/job/salt-pr-build/1929/

@ghost
Copy link

ghost commented Mar 2, 2014

Test FAILed.
Refer to this link for build results: http://jenkins.saltstack.com/job/salt-pr-build/1932/

@ghost
Copy link

ghost commented Mar 2, 2014

Test FAILed.
Refer to this link for build results: http://jenkins.saltstack.com/job/salt-pr-build/1933/

@ghost
Copy link

ghost commented Mar 2, 2014

Test FAILed.
Refer to this link for build results: http://jenkins.saltstack.com/job/salt-pr-build/1934/

@ghost
Copy link

ghost commented Mar 2, 2014

Test FAILed.
Refer to this link for build results: http://jenkins.saltstack.com/job/salt-pr-build/1935/

@kiorky
Copy link
Contributor Author

kiorky commented Mar 2, 2014

#10884 fixes one of the broken tests

@ghost
Copy link

ghost commented Mar 2, 2014

Test FAILed.
Refer to this link for build results: http://jenkins.saltstack.com/job/salt-pr-build/1937/

@ghost
Copy link

ghost commented Mar 2, 2014

Test FAILed.
Refer to this link for build results: http://jenkins.saltstack.com/job/salt-pr-build/1939/

@ghost
Copy link

ghost commented Mar 2, 2014

Test FAILed.
Refer to this link for build results: http://jenkins.saltstack.com/job/salt-pr-build/1942/

@ghost
Copy link

ghost commented Mar 2, 2014

Test FAILed.
Refer to this link for build results: http://jenkins.saltstack.com/job/salt-pr-build/1943/

@ghost
Copy link

ghost commented Mar 2, 2014

Test FAILed.
Refer to this link for build results: http://jenkins.saltstack.com/job/salt-pr-build/1944/

@kiorky
Copy link
Contributor Author

kiorky commented Mar 2, 2014

I think that 100% of the stuff is there, im now doing some tests, but review can be done.

Ping @thatch45 @s0undt3ch

@ghost
Copy link

ghost commented Mar 3, 2014

Test FAILed.
Refer to this link for build results: http://jenkins.saltstack.com/job/salt-pr-build/1952/

@s0undt3ch
Copy link
Member

Ok. My first observation is, can you submit the LXC stuff on another PR(even if one depends on the other, just mention it)? Does it really neend to be the same?

My other comments will come a little later since inline commenting from the phone is not that great.

@kiorky
Copy link
Contributor Author

kiorky commented Mar 3, 2014

Well, it will be tedious for me to locally have them separated as they are very dependant... Or if i inter-merge branches, then the history would be cluterred

This was referenced Mar 3, 2014
@kiorky
Copy link
Contributor Author

kiorky commented Mar 3, 2014

So i created #10897 & #10898 which on this PR is dependant, @s0undt3ch

return ret


def cloned(name,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a thought... would it make more sense to have a single present state that delegates to created and cloned?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the provision which does that is on salt-cloud, @mgwilliams.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it was does in state at a moment, but was removed and rebased for a long time :)

@ghost
Copy link

ghost commented Mar 5, 2014

Test FAILed.
Refer to this link for build results: http://jenkins.saltstack.com/job/salt-pr-build/2066/

import glob
import time
import signal
import logging
from pprint import pprint
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are you using this pprint import?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

uhm, not sure anymore

@ghost
Copy link

ghost commented Mar 6, 2014

Test FAILed.
Refer to this link for build results: http://jenkins.saltstack.com/job/salt-pr-build/2079/

@ghost
Copy link

ghost commented Mar 6, 2014

Test FAILed.
Refer to this link for build results: http://jenkins.saltstack.com/job/salt-pr-build/2092/

techhat added a commit that referenced this pull request Mar 6, 2014
[MUST BE CAREFULLY REVIEWED] saltcloud / lxc improvments
@techhat techhat merged commit 93d7ff2 into saltstack:develop Mar 6, 2014
@kiorky
Copy link
Contributor Author

kiorky commented Mar 6, 2014

/cc @regilero @jpcw @toutpt @Gagaro @jpoliva it's in!

@kiorky kiorky deleted the cloud_salt branch March 6, 2014 21:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants